Skip to content

fix(iam): tighten service-principal trust + SAML audience binding (bug-hunt 5.6/5.7)#2289

Merged
vieiralucas merged 2 commits into
mainfrom
bh-auth2
Jul 16, 2026
Merged

fix(iam): tighten service-principal trust + SAML audience binding (bug-hunt 5.6/5.7)#2289
vieiralucas merged 2 commits into
mainfrom
bh-auth2

Conversation

@vieiralucas

@vieiralucas vieiralucas commented Jul 16, 2026

Copy link
Copy Markdown
Member

Summary

Two auth-when-enabled correctness gaps from the 2026-07-15 bug hunt, both in fakecloud-iam.

  • 5.6 — Service-principal trust satisfiable by a look-alike role. A Principal: { Service: <host> } trust (and the STS service-linked-role assumption gate in assume.rs) matched any AssumedRole principal whose ARN merely contained the service host substring. A user could create and assume a role literally named lambda.amazonaws.com, and its assumed-role/lambda.amazonaws.com/... ARN would satisfy a Service: lambda.amazonaws.com trust — a privilege escalation. Now a genuine service principal is required: the ARN must carry the AWS-reserved aws-service-role/<service>/ path (which cannot be forged as an ordinary role name). New shared helper evaluator::arn_denotes_service.
  • 5.7 — SAML audience binding skippable. AssumeRoleWithSAML skipped audience validation entirely when (a) the named SAML provider was not registered (fall-through soft pass) or (b) a registered provider's assertion carried no audience claim. Now the provider must be registered (AWS validates the assertion against provider metadata, impossible for a non-existent provider), and when the provider metadata declares an audience (entityID) the assertion must present a matching one; a missing audience is rejected with InvalidIdentityToken.

Not in this PR (tracked separately)

  • 5.2 (cross-account AssumeRoot gated only by the caller's own organizations_root_sessions flag — no management-account / org-membership check) needs a new cross-crate organization-membership resolver, since IAM state carries no org topology today. That's its own unit of work and will land as a dedicated PR.

Test plan

  • cargo test -p fakecloud-iam --lib — 522 pass. New: service_principal_requires_service_linked_role_path, arn_denotes_service_matches_only_reserved_path, principal_service_matches_service_linked_role (genuine SLR allows + impostor denied), assume_role_with_saml_unregistered_provider_denied, assume_role_with_saml_missing_audience_rejected. Existing SAML tests updated to register a provider (AWS requires it).
  • cargo clippy -p fakecloud-iam --tests -- -D warnings clean.

No API surface / SDK / docs change (internal authorization behavior only).


Summary by cubic

Tightens service-principal trust matching and fixes SAML audience binding in fakecloud-iam to close bug-hunt 5.6 and 5.7. Blocks look‑alike roles from satisfying service trusts and enforces audience when provider metadata pins one.

  • Bug Fixes
    • Service principal trust: Principal: { Service: <host> } now matches only service-linked-role identities under aws-service-role/<service>/. Introduces evaluator::arn_denotes_service and uses it in the STS SLR assumption gate to prevent look‑alike roles.
    • SAML audience binding: AssumeRoleWithSAML keeps the soft pass for unregistered providers; when a registered provider’s metadata declares an audience (entityID), the assertion must include the same audience. Missing or mismatched audiences return InvalidIdentityToken.

Written for commit 3725a70. Summary will update on new commits.

Review in cubic

…g-hunt 5.6/5.7)

- 5.6 — a `Principal: { Service: <host> }` trust (and the service-linked-role
  assumption gate) matched any AssumedRole ARN that merely *contained* the
  service host string, so a user who created and assumed a role literally named
  e.g. `lambda.amazonaws.com` satisfied a Service trust — a privilege
  escalation. Now a genuine service principal is required: its ARN must carry
  the AWS-reserved `aws-service-role/<service>/` path (new
  `evaluator::arn_denotes_service`, shared by `principal_is_service` and the
  STS SLR-assumption gate).

- 5.7 — `AssumeRoleWithSAML` skipped audience binding for an unregistered SAML
  provider (fall-through soft pass) and for a registered provider when the
  assertion carried no audience claim. Now the named provider must be
  registered, and when its metadata declares an audience (`entityID`) the
  assertion must present a matching one; a missing audience is rejected.

Tests: evaluator unit tests (genuine SLR allows, look-alike/impostor denied);
STS unit tests (unregistered-provider denial, missing-audience rejection);
existing SAML tests updated to register a provider (AWS requires it).

5.2 (cross-account AssumeRoot org-membership / management-account gating) is
tracked as a separate PR — it needs a new cross-crate organization-membership
resolver (IAM state has no org topology today), which is its own unit of work.
@vieiralucas vieiralucas requested a review from Copilot July 16, 2026 00:41

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

…SAML

The 5.7 change required the named SAML provider to be registered, but the
conformance baseline (sts_assume_role_with_saml) deliberately assumes with an
unregistered provider and expects success — fakecloud tolerates that (there is
no provider metadata to bind an audience against anyway). Restore the soft-pass
for unregistered providers; keep the real fix: a REGISTERED provider whose
metadata declares an audience (entityID) still requires the assertion to carry
a matching audience (a missing/mismatched one is rejected). Drop the
now-invalid unregistered-provider-denied test.
@vieiralucas vieiralucas requested a review from Copilot July 16, 2026 03:27

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@vieiralucas vieiralucas merged commit 1e4a016 into main Jul 16, 2026
155 checks passed
@vieiralucas vieiralucas deleted the bh-auth2 branch July 16, 2026 05:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants